fix(sandbox): handle per-path Landlock errors instead of abandoning entire ruleset#677
Merged
johntmyers merged 2 commits intomainfrom Mar 30, 2026
Merged
Conversation
|
5595e3f to
25054ae
Compare
f1c4317 to
25054ae
Compare
…ntire ruleset A single missing path (e.g., /app in containers without that directory) caused PathFd::new() to propagate an error out of the entire Landlock setup closure. Under BestEffort mode, this silently disabled all filesystem restrictions for the sandbox. Changes: - Extract try_open_path() and classify_path_error() helpers that handle PathFd failures per-path instead of per-ruleset - BestEffort mode: skip inaccessible paths with a warning, apply remaining rules - HardRequirement mode: fail immediately on any inaccessible path - Add zero-rule safety check to prevent applying an empty ruleset that would block all filesystem access - Pre-filter system-injected baseline paths (e.g., /app) in enrichment functions so missing paths never reach Landlock - Add unit tests for try_open_path, classify_path_error, and error classification for ENOENT, EACCES, ELOOP, ENAMETOOLONG, ENOTDIR - Update user-facing docs and architecture docs with Landlock behavior tables, baseline path filtering, and compatibility mode semantics - Fix stale ABI::V1 references in docs (code uses ABI::V2) Closes #664
25054ae to
3bbdd34
Compare
NotFound errors for stale baseline paths (e.g. /app persisted in the server-stored policy but absent in this container) are expected in best-effort mode. Downgrade from warn! to debug! so the message does not leak into SSH exec stdout (the pre_exec hook inherits the tracing subscriber whose writer targets fd 1). Genuine errors (permission denied, symlink loops, etc.) remain at warn! for operator visibility. Also move custom_image e2e marker from /opt to /etc (a Landlock baseline read-only path) since the security fix now properly enforces filesystem restrictions.
pimlock
approved these changes
Mar 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/app) silently disables all Landlock restrictions underbest_effortmodehard_requirementenforcementRelated Issue
Closes #664
Changes
Core fix (
landlock.rs)try_open_path()helper that returnsOk(Some(fd))on success,Ok(None)+ warning inBestEffort, orErrinHardRequirementclassify_path_error()to produce accurate human-readable reasons for different failure types (ENOENT, EACCES, ELOOP, ENAMETOOLONG, ENOTDIR)rules_appliedcounter with zero-rule safety check — refuses to callrestrict_self()on an empty ruleset (which would block all filesystem access)info!after ruleset build showingrules_appliedandskippedcountsDefense-in-depth (
lib.rs)enrich_proto_baseline_paths()andenrich_sandbox_baseline_paths()usingPath::exists()best_effort) or errors (hard_requirement) at Landlock apply timeTests
try_open_pathbehavior for BestEffort/HardRequirement/valid paths,classify_path_errorfor 6 error typesDocumentation
docs/reference/policy-schema.md: Expanded Landlock section with compatibility mode behavior tabledocs/sandboxes/policies.md: New "Baseline Filesystem Paths" section explaining filtering behavior and user-path vs system-path distinctionarchitecture/security-policy.md: Per-path error handling, baseline path filtering, zero-rule safety check documentationarchitecture/sandbox.md: Updated Landlock isolation steps, added baseline path filtering noteABI::V1references (code usesABI::V2)Testing
cargo test -p openshell-sandbox— 330 tests pass (including 8 new Landlock tests)cargo fmt --all -- --check— cleancargo clippy -p openshell-sandbox— no new warningsChecklist